Skip to content

Conversation

ceelias
Copy link
Collaborator

@ceelias ceelias commented Jun 16, 2025

No description provided.

Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to talk through some of these things.

@@ -0,0 +1,74 @@
apiVersion: 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Provisioned" resources cannot be modified by users via Grafana. I think that's ok for dashboards since users can clone an existing dashboard, tweak it and save it with a new name. But for alerts, this might not be optimal since the original alert will still be there and potentially trigger at the wrong time. But we need/want to provide some OOTB alerts, so it's a tough call.

Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the mentions on VFL and the related airflow job are the only real issues.

@ceelias ceelias requested a review from gsmith-sas June 17, 2025 14:22
Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few nits.

Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ceelias ceelias merged commit 2a1cadd into main Jun 17, 2025
1 of 2 checks passed
@ceelias ceelias deleted the grafana-alerts branch June 17, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants